BUG: stop dict_generator from sampling initial_solution (#1109) - #1122
Conversation
|
I checked head
This validates both supported Environment: Python 3.12.6; NumPy 2.5.2; pytest 9.1.1; macOS 26.5.2 arm64. Current |
9cc93a1 to
fce9756
Compare
@ting-hong-shieh so in your opinion, should we proceed with this PR? |
Gui-FernandesBR
left a comment
Gui-FernandesBR
left a comment
There was a problem hiding this comment.
I dont see anything weird with this...
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #1122 +/- ##
===========================================
+ Coverage 82.18% 83.53% +1.35%
===========================================
Files 122 130 +8
Lines 16355 17086 +731
===========================================
+ Hits 13441 14273 +832
+ Misses 2914 2813 -101 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Based on what I verified at The current head is now |
…m#1109) Generate stochastic samples from declared constructor inputs only, and validate StochasticFlight.initial_solution on construct.
fce9756 to
dcc8c06
Compare
The nested a./b. list was indented deeper than the body of item 1 with no blank line in between, which docutils reports as "Unexpected indentation". Since #1122 landed this broke `build-docs` for every pull request opened against develop, including PRs that do not touch this file. Aligns the sublist with the parent item's body, adds the required blank line, and drops the now-unneeded line continuations.
RocketPy-Team#1122 made dict_generator walk the declared stochastic inputs rather than the whole instance, which is the right fix for RocketPy-Team#1109 and makes the collection-skip this branch carried redundant. add_cp_eccentricity and add_thrust_eccentricity run after __init__ has already built that list, so their values stopped being sampled: four eccentricities became none. They are declared as they are validated now. The component_collections mechanism is gone, since walking the declared inputs never saw the collections in the first place. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
) #1122 made dict_generator walk the inputs a model declared instead of every attribute on it, which is the right shape for #1109. add_cp_eccentricity and add_thrust_eccentricity run after __init__ has built that list, so their distributions stopped being drawn from: the value was set on the instance and every simulation used the same one, with nothing to say so. Bisected: at 3e16c9f all four eccentricities appear in the generated dictionary, at 5a71eb9 none of them do. An add_* method now declares what it installed, with the argument as given rather than the validated form, so _set_stochastic validates it again on each reseed and binds the distribution to the generator that is live then. ensemble_member was already fine, since StochasticEnvironment passes it through the constructor and the hasattr guard covers it not being set yet. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
3 participants
Pull request type
Checklist
black rocketpy/ tests//make lint) has passed locallypytest tests -m slow --runslow) have passed locallyCHANGELOG.md— no action needed; an LLM workflow auto-updates it after mergeCurrent behavior
StochasticModel.dict_generatortreated every instance tuple/list as a stochastic input, so a 14-tupleinitial_solutioncrashed ('float' object is not callable) and list form silently sampled one element.Fixes #1109
New behavior
dict_generatorsamples only declared constructor stochastic inputs.StochasticFlightvalidatesinitial_solutionon construct and never puts it in the generated dict.Breaking change
Additional information
Focused unit tests added/extended; full slow suite not run in this contribution pass.